Skip to content

DOC: Improve Index docstrings #36239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Sep 9, 2020
Merged

DOC: Improve Index docstrings #36239

merged 3 commits into from
Sep 9, 2020

Conversation

Nikhil1O1
Copy link
Contributor

@Nikhil1O1 Nikhil1O1 commented Sep 9, 2020 •
edited by dsaxton
Loading

Index is now described as List instead of Set

@Nikhil1O1
Update base.py
15350e9 
Index is now described as List instead of Set
@WillAyd WillAyd added the Docs label Sep 9, 2020
@WillAyd WillAyd added this to the 1.2 milestone Sep 9, 2020
@WillAyd
Copy link
Member

WillAyd commented Sep 9, 2020

Lgtm

@onshek
Copy link
Contributor

onshek commented Sep 9, 2020

Lgtm

Sorry for stepping in, I think it's not accurate enough according to #36170 (comment)

@Nikhil1O1
Copy link
Contributor Author

Some tests are failing... I don't recall changing anything else accept the documentation, Why is this even happening?
any idea how to fix this?

@dsaxton dsaxton changed the title Update base.py DOC: Improve Index docstrings Sep 9, 2020
@dsaxton
Copy link
Member

dsaxton commented Sep 9, 2020

Some tests are failing... I don't recall changing anything else accept the documentation, Why is this even happening?
any idea how to fix this?

The CI failures are unrelated to this change

dsaxton
dsaxton reviewed Sep 9, 2020
View reviewed changes
@Nikhil1O1 Nikhil1O1 mentioned this pull request Sep 9, 2020
Closed
5 tasks
@topper-123
Copy link
Contributor

In my opinion a set implies that values are unique, which isn’t true for indexes. Maybe “sequence” is better?

@dsaxton
Copy link
Member

dsaxton commented Sep 9, 2020

In my opinion a set implies that values are unique, which isn’t true for indexes. Maybe “sequence” is better?

@topper-123 That makes sense. I also think perhaps the whole thing could be rewritten to something like "Immutable sequence used for indexing and alignment." (remove references to ndarrays, sets, or being ordered since none apply). What do you think?

@Nikhil1O1
Copy link
Contributor Author

Looks more appropriate to me as well
Shall I improve it?
@dsaxton @topper-123

@dsaxton
Copy link
Member

dsaxton commented Sep 9, 2020

Rather than opening a separate PR can you make both changes here?

@Nikhil1O1
Copy link
Contributor Author

Sure

@Nikhil1O1
DOC: Improve Index docstrings
42ebc0c 
Discarded list/Set analogy. Will refer to it as Sequence
@Nikhil1O1
DOC: Improve Index docstrings
b8338ca 
Discarded list/Set analogy. Will refer to it as Sequence.
@Nikhil1O1
Copy link
Contributor Author

@dsaxton I did the required changes. This should do... Right?

@dsaxton dsaxton merged commit 6aa311d into pandas-dev:master Sep 9, 2020
@dsaxton
Copy link
Member

dsaxton commented Sep 9, 2020

Thanks @Nikhil1O1

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
@Nikhil1O1
DOC: Improve Index docstrings (pandas-dev#36239)
a4a73ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsaxton dsaxton dsaxton approved these changes

@topper-123 topper-123 topper-123 approved these changes

Assignees
No one assigned
Labels
Docs
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.

DOC: index described as a "set" but permits duplicates -- change to multiset or list?
5 participants
@Nikhil1O1 @WillAyd @onshek @dsaxton @topper-123